Pipes through statsContext to evaluateLiquid directive function #3024
Conversation
|
This PR makes changes to mapping-kit. Please ensure that the changes are reflected in the mapping-kit go library as well and link the PR in description. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3024 +/- ##
===========================================
+ Coverage 32.57% 80.06% +47.48%
===========================================
Files 14 1186 +1172
Lines 703 23877 +23174
Branches 119 4860 +4741
===========================================
+ Hits 229 19116 +18887
- Misses 474 4055 +3581
- Partials 0 706 +706 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
nick-Ag
changed the title
joe-ayoub-segment
dismissed stale reviews from silesky and varadarajan-tw
via
a1a0b87
|
Hi @nick-Ag I can't deploy this as Validate is failing. Can you take a look please? |
|
I'll remove the Ready for Release label for now. |
Tracks liquid metrics when batching as well WIP passes tags hopefully
…ole of import errors
nick-Ag
force-pushed
the
liquid-templates-metrics
branch
from
8d0c746 to
349c8cb
Compare
github-actions
bot
removed
mode:cloud
mode:device
deploy:registration
deploy:push
labels
|
marking this ready for release pending stratconn review - should be deployed asap |
There was a problem hiding this comment.
Pull Request Overview
This PR adds statistics tracking to Liquid template evaluation by threading a statsContext parameter through the mapping resolution pipeline. The change enables monitoring of Liquid template performance with metrics that include field-specific tags for better observability.
- Adds
statsContextparameter to core mapping functions to enable metric collection - Implements timing and success/failure tracking for Liquid template evaluations
- Enhances metrics with field-level tags to identify which fields trigger template evaluations
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/core/src/mapping-kit/liquid-directive.ts | Adds statsContext parameter and timing metrics for Liquid template evaluation |
| packages/core/src/mapping-kit/index.ts | Threads statsContext through mapping resolution pipeline and adds field-level tagging |
| packages/core/src/destination-kit/index.ts | Adds partnerAction tag to statsContext for better metric categorization |
| packages/core/src/destination-kit/action.ts | Passes statsContext to transform functions for batch and single operations |
|
|
||
| if (statsTagsExist) { | ||
| statsContext.tags = originalTags | ||
| } |
There was a problem hiding this comment.
The manual tag management creates potential for state corruption if an exception occurs during resolve(). Consider using a try/finally block or creating a scoped statsContext copy to ensure tags are always restored.
Copilot uses AI. Check for mistakes.
| } finally { | ||
| const duration = Date.now() - start | ||
| statsContext?.statsClient?.histogram('liquid.template.evaluation_ms', duration, [ | ||
| ...statsContext.tags, |
There was a problem hiding this comment.
This will throw a runtime error if statsContext.tags is undefined. The spread operator should be guarded: ...(statsContext.tags || [])
| ...statsContext.tags, | |
| ...(statsContext?.tags || []), |
Copilot uses AI. Check for mistakes.
|
Hi @nick-Ag PR deployed |
This PR supports the rollout of Liquid Templates by adding some stats tracking including tags for which field is triggering the metrics.
Testing
Tested successfully in stage by triggering a Liquid template evaluation in the action tester.
Metrics come through as expected